Fix FD leak, use-after-free, and pipe cleanup in server#27
Open
Waskim2 wants to merge 3 commits intoEndPositive:mainfrom
Open
Fix FD leak, use-after-free, and pipe cleanup in server#27Waskim2 wants to merge 3 commits intoEndPositive:mainfrom
Waskim2 wants to merge 3 commits intoEndPositive:mainfrom
Conversation
- Close pipe FDs (pipefd[0], pipefd[1]) in free_stream_context, previously only the socket FD was closed causing 2 FDs to leak per stream - Fix use-after-free in callback_close: server_ctx was freed before accessing server_ctx->thread_ctx - Close pipe write-end on stream fin so io_copy thread exits cleanly instead of hanging indefinitely Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
slipstream_server_poller threads were created on every prepare_to_send callback with no data available, without checking if one was already active for the stream. This caused hundreds of threads to accumulate over time, exhausting resources and eventually crashing the server. Add polling_active flag to stream context to ensure only one poller thread exists per stream at a time. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Stream contexts were never freed in most cases because picoquic_reset_stream() triggered a callback with stream_ctx=NULL, so the reset handler couldn't clean up. Now free_stream_context is called directly before reset in all error/close paths in prepare_to_send and stream_data callbacks: - ioctl failure - recv returning 0 (connection closed) - recv returning -1 (error) - write EPIPE (pipe closed) - write error This was the main source of FD/thread accumulation causing the server to become unresponsive after extended use. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
slipstream_server_free_stream_context()only closed the socket FD but never closedpipefd[0]andpipefd[1], leaking 2 file descriptors per stream. Over time this causedBad file descriptorerrors and connection failures.callback_close:server_ctxwas freed viaslipstream_server_free_context()before accessingserver_ctx->thread_ctxon the next line, leading to potential crashes/segfaults.pipefd[1]remained open, causing theslipstream_io_copythread to hang indefinitely onread().Reproduction
Connect a SlipNet client to a slipstream server with SOCKS target. After ~3 minutes of active use, the server logs fill with:
The connection drops and health checks fail. FD count grows continuously until the process becomes unusable.
Test plan
Bad file descriptorerrors no longer appear in logs during normal operation